[OpenVINO Backend] update getitem #21359

Mohamed-Ashraf273 · 2025-06-05T12:17:04Z

Hi @rkazants ,
I've updated __getitem__ using gather_nd as suggested, and I've avoided using convert_to_numpy. Also, I've enabled variables_test.
__getitem__ passed all tests aviable + I've tested it locally.
Here are my test cases:

import numpy as np
from keras import ops
from keras.src import testing

class GetItemOpsTest(testing.TestCase):
    def test_getitem(self):
        self.np_tensor = np.arange(24).reshape(2, 3, 4)
        self.tensor = ops.convert_to_tensor(self.np_tensor)

        t = self.tensor[1]
        n = self.np_tensor[1]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

        t = self.tensor[1, 2, 3]
        n = self.np_tensor[1, 2, 3]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

        t = self.tensor[1:2]
        n = self.np_tensor[1:2]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

        t = self.tensor[1:2, 2:3, 3:4]
        n = self.np_tensor[1:2, 2:3, 3:4]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

        t = self.tensor[1:2, None]
        n = self.np_tensor[1:2, None]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

        t = self.tensor[1:2, 2:3, ...]
        n = self.np_tensor[1:2, 2:3, ...]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

        t = self.tensor[1:2, ..., 3:4]
        n = self.np_tensor[1:2, ..., 3:4]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

        t = self.tensor[None, ..., 3:4, None]
        n = self.np_tensor[None, ..., 3:4, None]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

        t = self.tensor[1:2:None]
        n = self.np_tensor[1:2:None]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

        t = self.tensor[:, 2]
        n = self.np_tensor[:, 2]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

        t = self.tensor[None]
        n = self.np_tensor[None]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

        t = self.tensor[None, None]
        n = self.np_tensor[None, None]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

        t = self.tensor[...]
        n = self.np_tensor[...]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

        t = self.tensor[..., 1]
        n = self.np_tensor[..., 1]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

        t = self.tensor[..., 1, 2]
        n = self.np_tensor[..., 1, 2]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

        t = self.tensor[..., -1, 2]
        n = self.np_tensor[..., -1, 2]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

        t = self.tensor[..., -1:-2, 2]
        n = self.np_tensor[..., -1:-2, 2]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

        t = self.tensor[..., None, None]
        n = self.np_tensor[..., None, None]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

        t = self.tensor[None, ..., None]
        n = self.np_tensor[None, ..., None]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

        t = self.tensor[1, 2, None, ..., None]
        n = self.np_tensor[1, 2, None, ..., None]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

        t = self.tensor[None, ..., 1, 2]
        n = self.np_tensor[None, ..., 1, 2]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

        t = self.tensor[1, None, 2]
        n = self.np_tensor[1, None, 2]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

        index_tensor = ops.convert_to_tensor(np.array(1, dtype=np.int32))
        t = self.tensor[index_tensor]
        n = self.np_tensor[ops.convert_to_numpy(index_tensor)]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

        index_tensor = ops.convert_to_tensor(np.array(1, dtype=np.int32))
        t = self.tensor[index_tensor, 2, None]
        n = self.np_tensor[ops.convert_to_numpy(index_tensor), 2, None]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

        index_tensor = ops.convert_to_tensor(np.array(-2, dtype=np.int32))
        t = self.tensor[index_tensor, 1]
        n = self.np_tensor[ops.convert_to_numpy(index_tensor), 1]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

        index_tensor = ops.convert_to_tensor(np.array(-1, dtype=np.int32))
        t = self.tensor[-2, index_tensor]
        n = self.np_tensor[-2, ops.convert_to_numpy(index_tensor)]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

        # Negative indexing
        t = self.tensor[-1]
        n = self.np_tensor[-1]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

        t = self.tensor[1, -1, -2]
        n = self.np_tensor[1, -1, -2]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

        # Slicing with step
        t = self.tensor[::2]
        n = self.np_tensor[::2]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

        # Mixed slices and integers
        t = self.tensor[1, :, 1:4]
        n = self.np_tensor[1, :, 1:4]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

        t = self.tensor[:, 1:2, 3]
        n = self.np_tensor[:, 1:2, 3]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

The PR is ready for review.

codecov-commenter · 2025-06-05T12:23:05Z

Codecov Report

Attention: Patch coverage is 80.68182% with 17 lines in your changes missing coverage. Please review.

Project coverage is 82.71%. Comparing base (771b001) to head (48e69db).
Report is 18 commits behind head on master.

Files with missing lines	Patch %	Lines
keras/src/backend/openvino/core.py	80.68%	8 Missing and 9 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #21359      +/-   ##
==========================================
+ Coverage   82.70%   82.71%   +0.01%     
==========================================
  Files         565      565              
  Lines       54894    55150     +256     
  Branches     8517     8590      +73     
==========================================
+ Hits        45398    45619     +221     
- Misses       7415     7425      +10     
- Partials     2081     2106      +25

Flag	Coverage Δ
keras	`82.52% <80.68%> (+0.01%)`	⬆️
keras-jax	`63.41% <0.00%> (-0.16%)`	⬇️
keras-numpy	`58.60% <0.00%> (-0.12%)`	⬇️
keras-openvino	`33.71% <80.68%> (+0.18%)`	⬆️
keras-tensorflow	`63.82% <0.00%> (-0.14%)`	⬇️
keras-torch	`63.45% <0.00%> (-0.16%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

rkazants · 2025-06-08T16:52:03Z

@Mohamed-Ashraf273, can you add your tests to sources as well?

rkazants · 2025-06-08T16:54:28Z

keras/src/backend/openvino/core.py

+                    )
+                if index_type != Type.i32:
+                    index = ov_opset.convert(index, Type.i32).output(0)
+                shape_tensor = ov_opset.shape_of(data)


you can specify output type to i32 then you don't need convert below.

If I do that index = get_ov_output(index, Type.i32) it will not be changed to i.32 based on the implementation of get_ov_ouput:

keras/src/backend/openvino/core.py

Mohamed-Ashraf273 · 2025-06-08T17:58:05Z

@Mohamed-Ashraf273, can you add your tests to sources as well?

@rkazants
I added them to /keras/keras/src/ops/core_test.py

Mohamed-Ashraf273 · 2025-06-14T11:41:33Z

Hi @rkazants ,
I wanted to follow up and see if there are any updates.

Mohamed-Ashraf273 · 2025-06-16T12:47:29Z

Hi @rkazants ,
I have updated getitem as we discussed, and it is ready for review.

rkazants

@fchollet, looks good to me. Recommend to merge

google-ml-butler bot added the size:M label Jun 5, 2025

google-ml-butler bot assigned gbaned Jun 5, 2025

Mohamed-Ashraf273 force-pushed the update_getitem branch 5 times, most recently from 88938fd to 5043d78 Compare June 5, 2025 22:08

Mohamed-Ashraf273 marked this pull request as ready for review June 5, 2025 22:10

Mohamed-Ashraf273 marked this pull request as draft June 5, 2025 22:16

Mohamed-Ashraf273 force-pushed the update_getitem branch from 5043d78 to 91aa315 Compare June 5, 2025 22:40

Mohamed-Ashraf273 marked this pull request as ready for review June 5, 2025 22:40

Mohamed-Ashraf273 marked this pull request as draft June 5, 2025 22:49

Mohamed-Ashraf273 force-pushed the update_getitem branch 2 times, most recently from e7f4898 to 0ba9c10 Compare June 6, 2025 01:31

Mohamed-Ashraf273 marked this pull request as ready for review June 6, 2025 01:32

Mohamed-Ashraf273 force-pushed the update_getitem branch 3 times, most recently from d274a50 to 6ddb1b4 Compare June 6, 2025 12:30

Mohamed-Ashraf273 marked this pull request as draft June 6, 2025 12:34

Mohamed-Ashraf273 force-pushed the update_getitem branch 2 times, most recently from 4a1d931 to 08aebcf Compare June 6, 2025 20:26

Mohamed-Ashraf273 marked this pull request as ready for review June 6, 2025 20:27

rkazants reviewed Jun 8, 2025

View reviewed changes

keras/src/backend/openvino/core.py Outdated Show resolved Hide resolved

[OpenVINO Backend] update getitem

47c145c

Mohamed-Ashraf273 force-pushed the update_getitem branch from 08aebcf to 47c145c Compare June 8, 2025 17:55

Mohamed-Ashraf273 requested a review from rkazants June 11, 2025 12:05

google-ml-butler bot added the awaiting review label Jun 11, 2025

Mohamed-Ashraf273 force-pushed the update_getitem branch 3 times, most recently from 067e7de to 08daf79 Compare June 16, 2025 12:46

[OpenVINO backend] update getitem

48e69db

Mohamed-Ashraf273 force-pushed the update_getitem branch from 08daf79 to 48e69db Compare June 19, 2025 16:02

rkazants reviewed Jun 19, 2025

View reviewed changes

fchollet merged commit d16424e into keras-team:master Jun 20, 2025
7 checks passed

google-ml-butler bot removed the awaiting review label Jun 20, 2025

Mohamed-Ashraf273 deleted the update_getitem branch June 20, 2025 19:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[OpenVINO Backend] update getitem #21359

[OpenVINO Backend] update getitem #21359

Uh oh!

Mohamed-Ashraf273 commented Jun 5, 2025 •

edited

Loading

Uh oh!

codecov-commenter commented Jun 5, 2025 •

edited

Loading

Uh oh!

rkazants commented Jun 8, 2025

Uh oh!

rkazants Jun 8, 2025

Uh oh!

Mohamed-Ashraf273 Jun 8, 2025

Uh oh!

Uh oh!

Mohamed-Ashraf273 commented Jun 8, 2025

Uh oh!

Mohamed-Ashraf273 commented Jun 14, 2025

Uh oh!

Mohamed-Ashraf273 commented Jun 16, 2025

Uh oh!

rkazants left a comment

Uh oh!

Uh oh!

Uh oh!

[OpenVINO Backend] update __getitem__ #21359

[OpenVINO Backend] update __getitem__ #21359

Uh oh!

Conversation

Mohamed-Ashraf273 commented Jun 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented Jun 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

rkazants commented Jun 8, 2025

Uh oh!

rkazants Jun 8, 2025

Choose a reason for hiding this comment

Uh oh!

Mohamed-Ashraf273 Jun 8, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Mohamed-Ashraf273 commented Jun 8, 2025

Uh oh!

Mohamed-Ashraf273 commented Jun 14, 2025

Uh oh!

Mohamed-Ashraf273 commented Jun 16, 2025

Uh oh!

rkazants left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

[OpenVINO Backend] update getitem #21359

[OpenVINO Backend] update getitem #21359

Mohamed-Ashraf273 commented Jun 5, 2025 •

edited

Loading

codecov-commenter commented Jun 5, 2025 •

edited

Loading